-
Notifications
You must be signed in to change notification settings - Fork 303
✨ WIP: Add affinity/anti-affinity to VirtualMachine spec #3645
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release-1.14
Are you sure you want to change the base?
Conversation
- Bump VMOP including Node AF/AAF support - Add NodeAutoPlacement Feature Gate (cherry picked from commit 700c8ae)
Signed-off-by: Sagar Muchhal <[email protected]>
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
||
// Check the presence of the node-pool label on the VirtualMachineGroup | ||
nodePool := supervisorMachineCtx.Machine.Labels[clusterv1.MachineDeploymentNameLabel] | ||
if zone, ok := vmOperatorVMGroup.Labels[fmt.Sprintf("capv/%s", nodePool)]; ok && zone != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in capv design doc, adding prefix may will fail badly when using long node pool names. I've dropped the prefix in VMG controller.
} | ||
} | ||
|
||
// Check the presence of the node-pool label on the VirtualMachineGroup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: let's use Machine Deployment in capv instead of node pool.
// Check the presence of the node-pool label on the VirtualMachineGroup | ||
nodePool := supervisorMachineCtx.Machine.Labels[clusterv1.MachineDeploymentNameLabel] | ||
if zone, ok := vmOperatorVMGroup.Labels[fmt.Sprintf("capv/%s", nodePool)]; ok && zone != "" { | ||
supervisorMachineCtx.VSphereMachine.Spec.FailureDomain = ptr.To(zone) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For day 2 operations, we only need to create the VM with a label "topology.kubernetes.io/zone: Zone-x", but no need to set VSphereMachine.Spec.FailureDomain or Machine.Spec.FailureDomain. We should keep them consistent since they're empty from cluster specification.
After create VM with lable, VM Service will place it into that zone.
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #